Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update to match new UnityWebRequest API #709

Merged
merged 1 commit into from
Sep 9, 2017
Merged

Update to match new UnityWebRequest API #709

merged 1 commit into from
Sep 9, 2017

Conversation

haswalt
Copy link
Contributor

@haswalt haswalt commented Jul 31, 2017 •
edited
Loading

Description

Fixes support for Unity 2017.

Motivation and Context

Allows use with Unity 2017, fixes: #704, #701

Testing

I'm not a .NET developer and a on MacOS which means testing is awkward. Travis CI builds pass and the change is also very minor and only impacts Unity builds.

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project
  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have read the README document
  • I have added tests to cover my changes
  • All new and existing tests passed

License

  • I confirm that this pull request can be released under the Apache 2 license

@haswalt
Copy link
Contributor Author

haswalt commented Aug 3, 2017

Anyway to get these PRs bumped in faster? Currently this prevents the AWS SDK being used with Unity 2017 and there are quite a few bugs about it already that could be closed with this PR.

I can't add more tests or build locally for Unity as I'm on a mac and .Net Core doesn't allow me to build with net35 which Unity needs.

@oleschaper
Copy link

May I ask what is holding this back?

@normj
Copy link
Member

normj commented Sep 7, 2017

I am currently trying to get through our pull requests. Before accepting this I wanted understand first if this would be a breaking change for users not using 2017? Did we have the property wrong before or was it renamed in 2017?

@oleschaper
Copy link

It was renamed in Unity 2017.1, see
https://docs.unity3d.com/ScriptReference/Networking.UnityWebRequest.html
and
https://docs.unity3d.com/560/Documentation/ScriptReference/Networking.UnityWebRequest.html
So you might be right, for backward compatibility we would need to modify this change.

oleschaper
oleschaper suggested changes Sep 7, 2017
View reviewed changes
Copy link

@oleschaper oleschaper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PropertyInfo isErrorProperty = unityWebRequestType.GetProperty("isError");
if (isErrorProperty == null)
{
isErrorProperty = unityWebRequestType.GetProperty("isNetworkError");
}

@oleschaper
Copy link

oleschaper commented Sep 8, 2017 •
edited
Loading

tested with Unity 5.6.2, Unity 2017.1.0 and Unity 2017.1.1 on iOS and Android

@PavelSafronov PavelSafronov merged commit 87ba110 into aws:master Sep 9, 2017
@normj
Copy link
Member

normj commented Sep 9, 2017

Thanks for the pull request. Version 3.3.17.8 of the Core component contains this fix.

@normj normj mentioned this pull request Sep 10, 2017
Closed
@dkossnick-figma dkossnick-figma mentioned this pull request Sep 10, 2017
Closed
@barides
Copy link

barides commented Nov 12, 2017

Hi,

I would like to get Version 3.3.17.8, is there an option to get the distributable for this release or later. In the amazon website the download gives me reels 3.3.12.0 as the latest.

Can any one help out? I need the DLL with this fix.

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oleschaper oleschaper oleschaper requested changes

Assignees
No one assigned
Labels
Unity
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

NullReferenceException on Unity 2017 iOS
6 participants
@haswalt @oleschaper @normj @barides @PavelSafronov @sstevenkang